studio: document viewers for PDF, DOCX, PPTX, XLSX and CSV - #87

sequenceDiagram
  participant FileViewer
  participant LazyViewer
  participant AppProtocol
  participant DocumentViewer
  participant ElectronResource
  FileViewer->>LazyViewer: select viewer by FileType
  LazyViewer->>AppProtocol: request viewer WASM asset
  AppProtocol->>ElectronResource: read allowlisted WASM file
  ElectronResource-->>AppProtocol: return WASM bytes
  AppProtocol-->>DocumentViewer: serve WASM response
  DocumentViewer->>DocumentViewer: load and render document
  DocumentViewer-->>FileViewer: display controls and content

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands.}

Worked through both reviews. Fixed in d9e599e.

Real defects, all confirmed against the code before fixing:

• CSV zoom didn't resize rows. Verified in @tanstack/virtual-core@3.13.18 source rather than from the description: getMeasurements memoizes on [getMeasurementOptions(), itemSizeCache], and getMeasurementOptions() is {count, paddingStart, scrollMargin, getItemKey, enabled, lanes} — estimateSize genuinely isn't in it. measure() replaces itemSizeCache with a new Map, and that identity is a memo dependency, so it forces recomputation against the current zoom. Good catch; the grid was visibly broken at any zoom ≠ 1.
• DOCX revealPage global lookup. The sharpest finding of the set. The panel keeps its viewer mounted while the modal renders a second one for the same file, so document.querySelector always won the panel's copy and the modal's page navigation silently scrolled a hidden viewer. Now scoped to the viewer's own container, matching what useVisiblePage already did.
• Cached rejected engine promise — one transient wasm failure disabled PDF for the session and defeated the boundary's recover-on-next-file design.
• PDF document never closed on unmount — the engine is shared for the renderer's lifetime, so it held pdfium memory indefinitely.
• Shared wasm staging path. The existing comment claimed concurrency safety it didn't have; the staging name is now unique per writer.

Smaller ones taken: fallback card centering, inert on the collapsed rail, accessible names on the find controls and active sheet tab, debounced CSV find, zoom bounds derived from the level list.

Removed rather than kept: the modal's click-to-dismiss handler. Going full bleed made it unreachable, so it was dead code my own change created.

Two skipped, with reasons:

• satisfies on DOCUMENT_EXTENSIONS — tried it; it fails typecheck. Dropping the index signature makes the object's keys literal, so the arbitrary-string lookup at the call site errors with TS7053 (No index signature with a parameter of type 'string'). CodeRabbit hedged on exactly this, and the hedge was right. The annotation is load-bearing.
• The two security notes (wildcard CORS on the app scheme, CSP wasm-unsafe-eval + blob: workers) are accurate but not actionable here. Both are required by the wasm engines, the reasoning is documented at each site and in the plan, and the scheme is only reachable from the app's own web contents serving static version-shipped binaries. Worth revisiting if a host handling user data is ever added under that scheme — noted, not changed.

pnpm check-and-test:ci passes 22/22 and the production build is clean, with all four wasm binaries still landing in resources/wasm/.

One caveat on verification: I fixed these after my in-app pass, and couldn't re-drive the app afterwards — the DevTools daemon wedged. The CSV and virtualizer behavior I verified against library source instead, which is stronger evidence than a screenshot; the DOCX modal-scoping and PDF engine-retry paths are reasoned-through but not re-exercised by hand. They're on the unverified list in the PR description along with the packaged build.

Second round of feedback, all in. Four commits on top of the review fixes.

Real bugs found

XLSX was editable. readOnly is a property of the controller, not the viewer component: XlsxViewerInner reads its editing state from whichever controller it is handed and only builds one from its own props when none is supplied. We built the controller ourselves and passed readOnly to the component, so it did nothing — cell editing, paste and undo were all live against a workbook that never saves. Moved to useXlsxViewerController({ readOnly: true }) and dropped the inert prop. Verified there is no editor in the panel after a double-click.

XLSX zoom collapsed the sheet. setZoomScale takes Excel's percentage (DEFAULT_ZOOM_SCALE = 100, clamped 10..400), so handing it a factor of 1 clamped to the 10% floor — the tiny top-left view. Now level * 100, with the readout driven from controller.zoomScale so a trackpad pinch on the grid moves the toolbar number too, and min/max taken from the controller's own range.

PPTX didn't fill vertically. The height prop becomes the min and max height of the slide workspace, defaulting to a min(76vh, 780px) clamp — so it left the bottom empty however tall the panel was, and being a vh clamp it would have been wrong again at any zoom other than 1x. height="100%" resolves against the element inset-0 already sized.

Changes you asked for

Modal is no longer full bleed. Top inset is TOOLBAR_HEIGHT, which clears the whole toolbar and therefore the macOS traffic lights and the Windows caption buttons in one rule; 12px on the other three sides. It's the plain constant rather than one divided by the zoom, because the dialog content carries the same zoom factor the app root does — StudioToolbar's calc(5rem / var(--app-zoom)) is the opposite case, reserving room for OS-drawn buttons that are real pixels. Having a gutter again also makes click-to-dismiss reachable, so that's back.

Zoom is the app's own control. ZoomStepperControl — the one already shared by the settings row and the browser guest's page zoom — grew a compact size and an optional readout slot, and the viewers pass their level menu into it. Stepping now runs through the same steppedZoom ladder as the rest of the app, and the preset levels are a subset of that ladder so the two never disagree.

Find is the browser's own control. Extracted FindRow out of BrowserFindBar and used it verbatim inside the viewers' popover: same field, match readout, previous/next/close, same 1/97 format. The viewers keep the popover rather than an always-visible bar because the artifact panel can be too narrow to give a bar its own row — the rest is now literally the same component. Confirmed the browser bar still works end to end after the refactor (1/2 for "domain" on example.com).

Toolbar dividers are gone, replaced by spacing. The zoom stepper is already a bounded control with internal divisions, so the separators were stacking a second set of lines onto it.

DOCX has fit-width, computed from useDocxPageLayout's pageWidthPx against the scroll container. It's a one-shot rather than a mode, so the readout stays a number the stepper can move from — landed at 89% on demo.docx. Also started passing pageVirtualization={{ scrollElement, zoomScale }}, which the library asks for when zoom is controlled outside it; inference can't see a zoom the element sets on itself.

CSV lost its zoom and gained column sorting. Agreed on the zoom — it's plain DOM text the window zoom already scales, so a second scale factor was only a way for the two to disagree. Sorting is ascending → descending → file order, numeric when both cells parse as numbers (so 4.99 sorts before 12.50), blanks last in either direction.

Filename tooltip anchors to the name. It was flex-1, so the trigger box spanned the header and the tooltip appeared under the middle of it. Now a flex item that shrinks to its text: measured 66px for sample.pdf where it used to be ~494px.

Also verified

The DOCX modal-scoping fix from the last round, which I couldn't re-drive then: navigating to page 4 in the modal moved the modal's scroller to 3252 and left the panel's at 0.

Still not verified

Unchanged from before: a packaged build, the light theme, app zoom other than 1x, PDF text selection by hand, large files, and deliberately malformed files per format. Find remains unimplemented for DOCX and XLSX.

Third round. Six commits. The headline: PDF text selection already worked — what was missing was a way to get the text out, and a bitmap that hijacked the drag.

The PDF question, answered

I tested this rather than reasoning about it, because a whole second viewer hung on the answer.

pdfium extracts real text and SelectionLayer tracks a selection over it. Dragging across the title of attention.pdf produces a selection rect of exactly the right width. So the page is not just an image, and pdf.js would buy nothing here. Two things were hiding that:

1. The page bitmaps were drag sources. RenderLayer and TilingLayer both render an <img>, and an <img> drags by default. A press landing a few pixels off a glyph tore the page bitmap out as a drag image instead of starting a selection — I reproduced this by accident, dragging a PDF page into the chat composer as an attachment. Miss once and the feature looks absent. Both layers are pointer-events: none now.
2. Nothing bound a copy shortcut. CopyToClipboard only subscribes to an event the plugin never emits on its own. You could select and never take the text anywhere, which is exactly what "it's just an image" feels like. The viewer binds Cmd/Ctrl+C itself, guarded on the document actually holding selection rects — unguarded, copyToClipboard emits an empty string and would wipe the clipboard on any copy elsewhere in the app.

End to end now: select the title, Cmd+C, clipboard reads Attention Is All You Need.

So I have not built a pdf.js alternative behind a dev toggle. The premise it rested on turned out to be wrong, and a second engine is a lot of surface to carry. Say the word if you still want the comparison.

Fit width is a mode now, not a one-shot

You were right that it should track the splitter, and it wasn't the cluster of DOM listening you feared — one ResizeObserver in a shared hook. PDF already had this natively, which is why that one felt good; DOCX and PPTX now match it.

• PPTX fit was broken for a specific reason: zoom is a controlled prop, so whatever controller.setFitMode resolved was overwritten by our number on the next render. The fit has to be our number. Slide width comes from the deck's own size.widthEmu over 9525.
• DOCX opens fitted, since an 8.5in page overflows the panel at any usual width — opening at 100% meant opening on a horizontal scrollbar.
• Measured: dragging the splitter moved PPTX 757→823px / 76%→82%, and DOCX 823→523px / 97%→60%.

I also checked the app-zoom case you couldn't reproduce. At two steps of app zoom the PPTX slides stay fitted and centred (52%), nothing off-screen. The refit is the likely reason — but since I never reproduced the original, I can't claim it's fixed, only that I couldn't make it happen.

Right-click

PDF, PPTX and XLSX now show the file's own actions. All three paint their content as images, so Chromium was offering Save Image As on one rasterized page. DOCX and CSV keep the native menu deliberately: those are real DOM text and Copy on a selection is the right offer there — verified that a right-click in the DOCX viewer still goes native.

Loading state

Skeleton instead of the spinner. Kept it a plain block rather than a page or grid mock-up: the viewers using it land on all three shapes and a wrong guess is worse than none.

.numbers

Not supported and not cheap to add. @dukelib/sheets-wasm reads OOXML and legacy .xls; Apple's format is a proprietary undocumented container, so it would need its own parser. Those files keep falling through to the associated app.

Two review comments I'd missed

Devin caught a real regression I introduced: widening FileType with csv/docx/pptx/xlsx silently changed three predicates that matched on the old values. Office and CSV attachments dropped from full-width preview rows to 48px chips, CSV lost its text-lines thumbnail, and CSV lost the Copy action. Directly contradicts "the Files panel stays exactly as it is on main" in the plan. All three are now exhaustive Record<FileType, boolean> tables, so the next added type is a compile error rather than a silent presentation change.

Also dropped canPreviewFile: its comment described OS-routing that nothing implements, and no production code called it. Its test assertions moved onto getFileType, where they test the real behaviour.

Not done, on purpose

The DOCX re-parse when opening the expand modal — you said not to bother, and caching a parsed model across component instances is real work for a spinner you don't care about.

Still unverified

Packaged build, light theme, PDF selection across a page boundary and at several zoom levels, large files, malformed files per format. pnpm check-and-test:ci passes 22/22.